-
Notifications
You must be signed in to change notification settings - Fork 7
Add support for Presence and Refactor SDK architecture #404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a comprehensive service lifecycle management system and WebSocket-based presence functionality to the LootLocker Unity SDK. The changes establish centralized service coordination through a new LootLockerLifecycleManager and add real-time presence capabilities with platform-specific optimizations, including battery-aware mobile support and automatic session management.
Key Changes:
- Introduced
LootLockerLifecycleManagerfor centralized service initialization, dependency management, and Unity lifecycle event coordination - Added
LootLockerEventSystemfor typed, thread-safe event handling with automatic memory leak prevention via weak references - Implemented WebSocket-based presence system (
LootLockerPresenceManagerandLootLockerPresenceClient) with platform-specific configurations and battery optimizations
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
LootLockerConfig.cs |
Added presence platform configuration enums and helper methods for platform detection |
RemoteSessionRequest.cs |
Converted RemoteSessionPoller to implement ILootLockerService interface with lifecycle management |
LootLockerSDKManager.cs |
Integrated lifecycle manager initialization and added presence API methods |
ProjectSettings.cs |
Added presence configuration UI with platform toggles and battery optimization settings |
LootLockerAdminExtension.cs |
Updated cleanup to use lifecycle manager instead of direct HTTP client reset |
LootLockerStateData.cs |
Added event system integration for automatic session state management |
LootLockerServerApi.cs |
Converted to service architecture with lifecycle manager integration |
LootLockerRateLimiter.cs |
Implemented ILootLockerService interface with proper reset functionality |
LootLockerPresenceManager.cs |
New presence manager service for WebSocket connection coordination |
LootLockerPresenceClient.cs |
New WebSocket client implementation with connection management and latency tracking |
LootLockerLifecycleManager.cs |
New centralized service lifecycle coordinator with dependency management |
LootLockerHTTPClient.cs |
Converted to service architecture with improved memory management and queue limits |
LootLockerEventSystem.cs |
New centralized event system with weak reference-based subscriptions |
LootLockerEndPoints.cs |
Added presence WebSocket endpoint definition |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3ac9f74 to
ed2e29f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
Runtime/Client/LootLockerPresenceManager.cs:1
- The
LootLockerPresenceManagerreferencesStartGooglePlayGamesSessionevent handling but the session started event trigger is missing for this authentication method. In the SDK manager diff at line 1116,StartGooglePlayGamesSessioncreates playerData but doesn't callLootLockerEventSystem.TriggerSessionStarted(playerData), unlike other authentication methods in the same file.
#if LOOTLOCKER_ENABLE_PRESENCE
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JohannesLoot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very minor things, other than that it is all approved.
Great job on this one 👍
kirre-bylund
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
54a5b27 to
de3df4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5600b56 to
e4dd867
Compare
kirre-bylund
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed only to LootLockerLifeCycleManager so far
kirre-bylund
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed only to LootLockerLifeCycleManager so far
tigran-sargsyan-w
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, just dropping a quick drive-by review 👋
Overall the new presence-related changes look consistent with the rest of the SDK, and the public surface seems easy enough to follow from a user point of view.
A couple of small thoughts:
- If it’s not already covered somewhere else, it might be nice to add a short usage example to the docs / samples next to the other player-related APIs so people can discover this more easily.
- Same for tests: if there are any edge cases around presence state transitions / error handling, a couple of focused tests could help catch regressions later.
Other than that, I didn’t notice anything that looked blocking from my side. 👍
018729f to
ae04ad3
Compare
Tests/LootLockerTestUtils/LootLockerTestConfigurationTitleConfig.cs
Outdated
Show resolved
Hide resolved
This reverts commit b3a7d67.
ae04ad3 to
297b293
Compare
Add LootLocker Lifecycle Manager and Presence System
📋 Summary
This PR introduces a comprehensive service lifecycle management system and WebSocket-based presence functionality to the LootLocker Unity SDK. The changes establish a robust foundation for service coordination while adding real-time presence capabilities with platform-specific optimizations.
🎯 Key Features
LootLocker Lifecycle Manager
LootLockerLifecycleManagercoordinates all SDK services with proper initialization order and dependency managementILootLockerServiceinterface for consistent service lifecycle patternsEvent System
LootLockerEventSystemprovides typed, thread-safe event handling across the SDKPresence System
LootLockerPresenceClientandLootLockerPresenceManagerfor real-time presence features🔧 Technical Improvements
Performance Optimizations
Architecture Enhancements
Developer Experience
🚀 API Changes
New Public APIs
Enhanced APIs
🔧 Breaking Changes
None - This PR is designed to be fully backward compatible. All existing APIs continue to work exactly as before.
🧪 Testing Considerations
Service Lifecycle
Presence System
Event System
📱 Platform Support
Presence Platform Matrix
Configuration
Presence can be configured in Project Settings > LootLocker SDK > Presence Settings with platform-specific toggles and battery optimization options.
📋 Commit Structure
This PR includes 7 atomic commits for easier review:
🔍 Review Notes
🚀 Migration Guide
For Existing Users
No migration required - all existing code continues to work unchanged.
For New Presence Features
For Event System
✅ Checklist
This PR establishes a solid foundation for future SDK enhancements while adding powerful real-time capabilities that developers have been requesting.